keep/api/tasks/process_incident_task.py (3)

35-38: LGTM! Consider minor improvements to context extraction

The addition of job context information is valuable for tracking processing attempts. The implementation is correct, but we can make it slightly more concise.

    if ctx and isinstance(ctx, dict):
        extra["job_try"] = ctx.get("job_try", 0)
-       extra["job_id"] = ctx.get("job_id", None)
+       extra["job_id"] = ctx.get("job_id")

🧰 Tools

🪛 Ruff

37-37: Use ctx.get("job_id") instead of ctx.get("job_id", None)

Replace ctx.get("job_id", None) with ctx.get("job_id")

(SIM910)

---

Line range hint 156-158: Consider implementing exponential backoff for retries

The current linear retry delay (job_try * TIMES_TO_RETRY_JOB) might not be optimal for handling transient failures. Consider implementing exponential backoff to provide shorter initial retries while still allowing longer delays for persistent issues.

        # Retrying only if context is present (running the job in arq worker)
        if bool(ctx):
-           raise Retry(defer=ctx["job_try"] * TIMES_TO_RETRY_JOB)
+           # Exponential backoff with base of 2 minutes
+           raise Retry(defer=2 ** ctx["job_try"])

🧰 Tools

🪛 Ruff

37-37: Use ctx.get("job_id") instead of ctx.get("job_id", None)

Replace ctx.get("job_id", None) with ctx.get("job_id")

(SIM910)

---

Line range hint 101-104: Consider more specific exception handling for alerts processing

The current broad exception handling for alerts processing might mask specific issues. Consider:

1. Catching specific exceptions that you expect might occur
2. Adding a flag or status to the incident to indicate incomplete alert processing
3. Including alert processing status in the pusher notification

Example implementation:

try:
    # ... alerts processing code ...
except (ValueError, KeyError) as e:
    logger.exception("Error processing alert data: %s", str(e), extra=extra)
except ConnectionError as e:
    logger.exception("Network error processing alerts: %s", str(e), extra=extra)
    incident_from_db.alert_processing_status = "incomplete"
except Exception as e:
    logger.exception("Unexpected error adding incident alerts: %s", str(e), extra=extra)
    incident_from_db.alert_processing_status = "failed"

🧰 Tools

🪛 Ruff

37-37: Use ctx.get("job_id") instead of ctx.get("job_id", None)

Replace ctx.get("job_id", None) with ctx.get("job_id")

(SIM910)

keep/api/routes/incidents.py (1)

Line range hint 1012-1034: Fix response model inconsistency

The endpoint decorator specifies response_model=List[AlertDto], but the function returns a Response(status_code=202). This mismatch could cause issues with OpenAPI documentation and client expectations.

Apply this fix:

@router.post(
    "/{incident_id}/alerts",
    description="Add alerts to incident",
    status_code=202,
-   response_model=List[AlertDto],
)
async def add_alerts_to_incident(
    incident_id: UUID,
    alert_ids: List[UUID],
    is_created_by_ai: bool = False,
    authenticated_entity: AuthenticatedEntity = Depends(
        IdentityManagerFactory.get_auth_verifier(["write:incident"])
    ),
    pusher_client: Pusher | None = Depends(get_pusher_client),
    session: Session = Depends(get_session),
):
    tenant_id = authenticated_entity.tenant_id
    incident_bl = IncidentBl(tenant_id, session, pusher_client)
    await incident_bl.add_alerts_to_incident(incident_id, alert_ids)
    return Response(status_code=202)

Configuration used: CodeRabbit UI
Review profile: CHILL